Skip method this type validity filter for objects with more than 20 members - #28692
Conversation
Sheetal Nandi (sheetalkamat)
left a comment
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
Could this be confusing from user perspective? I am wondering what we are achieving anything significant through filtering. To me it seems the filtering helps when there are too many options but here we are limiting when there are fewer options so seems contradictory. Change looks good but will let Daniel Rosenwasser (@DanielRosenwasser) comment on whether we should skip this type check all the time or with the condition you have.
| if (typeChecker.isValidPropertyAccessForCompletions(node.kind === SyntaxKind.ImportType ? <ImportTypeNode>node : <PropertyAccessExpression>node.parent, type, symbol)) { | ||
| const props = type.getApparentProperties(); | ||
| for (const symbol of props) { | ||
| if (typeChecker.isValidPropertyAccessForCompletions(node.kind === SyntaxKind.ImportType ? <ImportTypeNode>node : <PropertyAccessExpression>node.parent, type, symbol, props.length > 20)) { |
There was a problem hiding this comment.
Make 20 a constant for better readability and maintainability
|
It's probably worth noting that at least Ron Buckton (@rbuckton) apparently has a library that relies on this filtering for more accurate method completions. :S |
Ryan Cavanaugh (RyanCavanaugh)
assigned
Wesley Wigham (weswigham) and unassigned
Wesley Wigham (weswigham)
|
Superseded by #31377 |
2 participants
Fixes #23285 specifically by limiting the number of properties we filter methods on to 20 (so if an object has > 20 members, we no longer filter invalid method calls from the completion list), though the underlying issue remains. It's also questionable if the filtering is even really correct, since using the member in a non-calling fashion is still acceptable - the choice to filter methods at all feels somewhat pragmatic.
That underlying issue is that every anonymous type instantiation is unique - so even though we're querying similar types across the ~300 this-type comparisons we perform, since the input anonymous types are differing identities, we repeat a lot of very similar work. Caching on signatures doesn't help here, since each comparison is triggered for a different signature. The issue is simply that if I have 200 methods with
this: {item: T}, then every time we infer from{item: string}to{item: T}and instantiateTwithstring, we get a new overall type identity and need to perform a structural comparison, even though it's still just{item: string}.